Replace the duplicated UART NS16550 console code with a shared driver - #900
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Refactors duplicated PC16552D/NS16550 UART console implementations across NXP QorIQ (PPC) and LS1028A (AArch64) targets into a shared instance-based NS16550 driver, with unit tests validating the shared behavior.
Changes:
- Introduces a generic instance-based NS16550 UART driver and optional platform MMIO accessors.
- Migrates NXP PPC and LS1028A console paths to the shared driver while keeping a minimal stage1-only fallback on size-constrained builds.
- Adds host-based unit tests exercising divisor math, register spacing, CRLF handling, and bounded waits.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/unit-tests/unit-ns16550.c | Adds host-emulated unit tests for the new NS16550 driver |
| tools/unit-tests/Makefile | Registers and builds the new NS16550 unit test |
| test-app/Makefile | Ensures test-app links the shared NS16550 object for affected targets |
| stage1/Makefile | Adds build rule/vpath support for sources under hal/uart |
| options.mk | Adds NS16550 build option to include the driver independent of DEBUG_UART |
| include/nxp_ppc_io.h | Adds PPC-specific ordered MMIO accessor hooks for byte IO |
| include/ns16550.h | Adds the public instance-based NS16550 driver API and IO hooks |
| hal/uart/ns16550.c | Implements the shared instance-based NS16550 driver |
| hal/nxp_t2080.h | Removes duplicated UART register macros now covered by shared driver |
| hal/nxp_t2080.c | Replaces UART_MCR macro usage with explicit DUART offset write |
| hal/nxp_t10xx.c | Removes duplicated UART register macros now covered by shared driver |
| hal/nxp_ppc.c | Switches PPC loader console to shared driver; keeps stage1 minimal UART |
| hal/nxp_p1021.c | Removes duplicated UART register macros now covered by shared driver |
| hal/nxp_ls1028a.h | Fixes DUART stride and removes duplicated UART register macros |
| hal/nxp_ls1028a.c | Switches LS1028A console to shared driver (no floating point divisor) |
| arch.mk | Links NS16550 object and sets PPC IO hook macro where needed |
| .gitignore | Ignores the newly added unit-ns16550 test binary |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
dgarske
force-pushed
the
ns16550_dedup
branch
from
September 18, 2026 01:28
95220ae to
168d504
Compare
danielinux
approved these changes
Sep 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
wolfBoot carried four copies of the same PC16552D console: a shared PowerPC implementation in
hal/nxp_ppc.creaching T2080, T1024, T1040 and P1021, a verbatim fork of it inhal/nxp_ls1028a.cfor AArch64, and three byte-identical register tables innxp_t2080.h,nxp_t10xx.candnxp_p1021.c. All of them now use one driver.Bugs fixed
Two latent bugs in the LS1028A fork, which the shared code removes:
UART_BASE(n)used a decimal100where the DUART stride is0x100. Harmless only becauseUART_SELis 0.Hardware testing
LS1028ARDB (the AArch64 fork) the console output is byte-for-byte identical before and after.
T1040D4RDB (the shared
hal/nxp_ppc.cpath) every wolfBoot line is identical through HAL init, QE/FMAN microcode upload, PHY enumeration, multicore start, image verification, the FDT fixups and handoff; the only deltas are the test app's own size, since it links the refactored HAL.T2080 and P1021 are build-only, including
OS_64BIT=1: Unit tests cover the divisor arithmetic,reg-shift/reg-offsetaddressing, CRLF and the bounded busy-waits against an emulated register block.